Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@bors r+

📌 Commit d54c3ea has been approved by bjorn3

It is now in the queue for this repository.

Looks like some GDB tests failed #143755 (comment)
@bors r-
@bors2 try jobs=dist-i586-gnu-i586-i686-musl

⌛ Trying commit d54c3ea with merge f7ba1a6…

To cancel the try build, run the command @bors2 try cancel.

💔 Test failed

• CI ❌

Looks like some GDB tests failed #143755 (comment)

Interesting, I'm going to have a look. The test passes on my x86_64-unknown-linux-gnu; I'll try to find what's different in the CI job. The name dist-i586-gnu-i586-i686-musl suggests that it's a 32-bit build - where can I find out more? Is there a way to run the same setup locally?

You might be able to get away with passing --target ... to ./x since these tests should still be able to run on your host. It looks like it fails testing i686-unknown-linux-musl.

If that doesn't repro it for some reason you can replicate the CI setup pretty much exactly by running the docker tests https://rustc-dev-guide.rust-lang.org/tests/docker.html, with the downside that this takes about as long as CI does.

Hmm, the full log of the CI job led me to believe that it's i586-unknown-linux-gnu, but locally the test passes for that target (running ./x.py test tests/debuginfo/embedded-visualizer.rs --target i586-unknown-linux-gnu). Will try with the musl target 👍

I noticed though that my GDB is version 15, whereas the test log shows GDB 12; maybe that's the reason for the discrepancy. In general, the .debug_gdb_scripts section looks correct to me...

You're right, it's i586. I must have been reading the wrong line.

##[group]Testing stage2 compiletest suite=debuginfo mode=debuginfo (x86_64-unknown-linux-gnu -> i586-unknown-linux-gnu)

I assume there probably isn't any reason we need to keep a very old gdb version if that makes a difference

@bors rollup=iffy
this may bounce a couple more times

⌛ Testing commit 8be915f with merge f7d013e...

💔 Test failed - checks-actions

Please add an exception to tests/run-make/symbols-all-mangled/rmake.rs. The symbol is actually mangled. It just doesn't start with _ZN or _R like most mangled symbols.

☔ The latest upstream changes (presumably #143919) made this pull request unmergeable. Please resolve the merge conflicts.

This PR modifies run-make tests.

cc @jieyouxu

Added the exception and rebased on master.

@bors r+

📌 Commit bc1ff10 has been approved by bjorn3

It is now in the queue for this repository.

Hmm, there's some problem with the omit_gdb_pretty_printer_section attribute now, at least in my local tests 😳 Do we get the section from somewhere even if it's omitted from the main crate? I suppose this could happen if the test linked against one of the crate types that are considered leaves in needs_gdb_debug_scripts_section, like dylib. But shouldn't those have had pretty printers before as well? Maybe the linker removed them because there's no main shim in those crate types 🤔

Tests get compiled with -Cprefer-dynamic by default to dynamically link against libstd. Try adding //@ no-prefer-dynamic to the relevant tests.

@bors r-

Try adding //@ no-prefer-dynamic to the relevant tests.

That doesn't seem to help - we get the section from the libstd rlib as well. It seems to be part of all object files inside the archive. I wonder why...

Oh, I think it's because of this bit in library/std/Cargo.toml:

[lib]
crate-type = ["dylib", "rlib"]

The code in needs_gdb_debug_scripts_section checks whether any of the crate types needs the section! That doesn't seem right, but reading the comment in that function made me wonder about the approach more generally:

// To ensure the section __rustc_debug_gdb_scripts_section__ will not create
// ODR violations at link time, this section will not be emitted for rlibs since
// each rlib could produce a different set of visualizers that would be embedded
// in the .debug_gdb_scripts section. For that reason, we make sure that the
// section is only emitted for leaf crates.

This doesn't apply anymore, does it? There's no risk of ODR violations because the symbols get a unique name. The real reason now is that get_or_insert_gdb_debug_scripts_section_global still collects visualizers transitively from all crates. If we keep the transitive collection, shouldn't we drop the emission of the debug script section for dylibs? I'm thinking that they shouldn't have the section because the compiler can see their visualizers when building executables that use them, can't it? I've tested locally that returning false for CrateType::Dylib fixes the debuginfo tests, but I don't know whether it has unwanted side effects.

Alternatively, we could collect visualizers for the current crate only, emit the per-crate list for all crate types, and rely on the linker to merge the .debug_gdb_scripts sections. This would require special handling of gdb_load_rust_pretty_printers.py though - maybe it could be emitted only by libstd?

the compiler can see their visualizers when building executables that use them

This seems to be incorrect when the dependency is in a dylib crate. I tried a variant of tests/debuginfo/embedded-visualizer.rs where the dependency is built as a dylib instead of an rlib. When I don't emit the .debug_gdb_scripts section for dylibs, the test fails because the dependency's visualizer is missing. It works fine, however, with an rlib, even if there's no .debug_gdb_scripts section in it.

Does this mean that the compiler has more visibility into rlib dependencies than into dylibs?

Ah, here we go: collect_debugger_visualizers_transitive filters by metadata source, apparently excluding dylibs. If I remove the filter and stop the compiler from emitting .debug_gdb_scripts for dylibs, all debuginfo tests pass 🥳

@rustbot ready